Skip to content

Conversation

@ibigbug
Copy link
Member

@ibigbug ibigbug commented Jun 14, 2025

🤔 This is a ...

  • New feature
  • Bug fix
  • Performance optimization
  • Enhancement feature
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Changelog is provided or not needed

@ibigbug ibigbug requested a review from Copilot June 14, 2025 18:46

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Jun 14, 2025

Codecov Report

Attention: Patch coverage is 82.35294% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
clash_lib/src/app/inbound/manager.rs 88.00% 3 Missing ⚠️
clash_lib/src/app/inbound/network_listener.rs 0.00% 0 Missing and 2 partials ⚠️
clash_lib/src/app/api/handlers/config.rs 85.71% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@ibigbug ibigbug requested a review from Copilot June 14, 2025 19:14

This comment was marked as outdated.

@ibigbug ibigbug requested a review from Copilot June 14, 2025 19:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR exposes the allow-lan setting via the HTTP API by making it reloadable at runtime.

  • Added an endpoint test to verify getting and setting allow-lan
  • Made allow_lan reloadable in InboundManager (with get_allow_lan / set_allow_lan)
  • Updated API handlers to include allow_lan in GET / PATCH /configs

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
clash_lib/tests/api_tests.rs New async test for GET and PATCH of allow-lan
clash_lib/src/config/internal/listener.rs Removed outdated TODO comment on reloadability
clash_lib/src/app/inbound/network_listener.rs Moved info! logs into spawned tasks
clash_lib/src/app/inbound/manager.rs Added get_allow_lan / set_allow_lan and restart logic
clash_lib/src/app/api/handlers/config.rs Included allow_lan in API GET and PATCH handlers
Comments suppressed due to low confidence (2)

clash_lib/tests/api_tests.rs:64

  • The format! call uses a named placeholder {configs_url} without passing a corresponding argument, causing a compile error. Change it to a positional placeholder ({} {}) or supply configs_url = configs_url.
                "application/json' -d '{\"allow-lan\": false}}' {configs_url}",

clash_lib/src/app/api/handlers/config.rs:210

  • Combining if let Some(...) = ... && ... is invalid Rust syntax. Consider nesting or matching separately, e.g. if let Some(v) = payload.allow_lan { if v != inbound_manager.get_allow_lan().await { ... } }.
    if let Some(allow_lan) = payload.allow_lan

@ibigbug ibigbug merged commit 300b1ce into master Jun 14, 2025
34 of 35 checks passed
@ibigbug ibigbug deleted the set-allow-lan branch June 14, 2025 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants